[PWGLF] dndeta-hi: move from PWGMM, add SD/DD/ND classification#15779
Draft
hyungjun-lee wants to merge 9 commits intoAliceO2Group:masterfrom
Draft
[PWGLF] dndeta-hi: move from PWGMM, add SD/DD/ND classification#15779hyungjun-lee wants to merge 9 commits intoAliceO2Group:masterfrom
hyungjun-lee wants to merge 9 commits intoAliceO2Group:masterfrom
Conversation
Contributor
hyungjun-lee
commented
Apr 15, 2026
hyungjun-lee
commented
- Move dndeta-hi.cxx from PWGMM/Mult/Tasks to PWGLF/Tasks/GlobalEventProperties
- Add kDD, kSD, kND enum values for diffractive event classification
- Join McCollisions with HepMCXSections to access processId() in processMCCounting
- Classify events by Pythia8 process code: 101=ND, 103/104=SD, 105/106=DD
- Replace sliceByCached with sliceBy+Preslice for soa::Filtered tables
- Fix std-prefix: std::log, std::exp, std::fabs
- Fix const-ref-in-for-loop: const auto& in range-based for loops
- Fix pdg/explicit-code: 310->kK0Short, 3122->kLambda0
- Add CMakeLists.txt entry for dndeta-hi workflow
- Move dndeta-hi.cxx from PWGMM/Mult/Tasks to PWGLF/Tasks/GlobalEventProperties - Add kDD, kSD, kND enum values for diffractive event classification - Join McCollisions with HepMCXSections to access processId() in processMCCounting - Classify events by Pythia8 process code: 101=ND, 103/104=SD, 105/106=DD - Replace sliceByCached with sliceBy+Preslice for soa::Filtered tables - Fix std-prefix: std::log, std::exp, std::fabs - Fix const-ref-in-for-loop: const auto& in range-based for loops - Fix pdg/explicit-code: 310->kK0Short, 3122->kLambda0 - Add CMakeLists.txt entry for dndeta-hi workflow
hyungjun-lee
requested review from
abmodak,
alibuild,
gbencedi,
mpuccio,
omvazque and
skundu692
as code owners
|
O2 linter results: ❌ 1 errors, |
Collaborator
|
Error while checking build/O2Physics/o2 for f7ae6fe at 2026-04-15 10:25: Full log here. |
Collaborator
|
Error while checking build/O2Physics/o2 for 400b75d at 2026-04-15 10:56: Full log here. |
Collaborator
|
Hi @hyungjun-lee, can you please take care of o2-building error (https://ali-ci.cern.ch/alice-build-logs/AliceO2Group/O2Physics/15779/400b75d0b31bdf93b26a9e7717525813cfec57df/build_O2Physics_o2/pretty.html) by removing the line Thanks, |
Contributor
Author
|
The remaining o2linter error originates from code outside of my responsibility. Could you please approve this commit? |
Collaborator
|
Hello @hyungjun-lee, I talked about the O2-building errors (not the linter error). But anyway, you fixed it, thanks! Thanks, |
Collaborator
|
Error while checking build/O2Physics/o2 for 2d7e2be at 2026-04-15 11:40: Full log here. |
Collaborator
|
@hyungjun-lee You are supposed to test your changes before making a PR. Your code contains obvious errors which make the compilation fail. How did you test your changes? |
Contributor
Author
|
@vkucera hello, Vit. It appears that parts affecting the build were inadvertently modified while updating the code to pass O2linter. The previous version of the code had been thoroughly tested in the local environment. I have now corrected the relevant sections and re-uploaded the code. |
Collaborator
|
I don't think your list of includes is correct. Please fix it. See https://aliceo2group.github.io/analysis-framework/docs/tools/#cleaning-include-statements-and-using-statements |
vkucera
reviewed
Apr 16, 2026
Comment on lines
+81
to
+90
| enum { | ||
| kECbegin = 0, | ||
| kDATA = 1, | ||
| kINEL, | ||
| kINELg0, | ||
| kDD, | ||
| kSD, | ||
| kND, | ||
| kECend | ||
| }; |
Collaborator
There was a problem hiding this comment.
- Give the
enuma name. - Don't use the
kprefix. It violates the naming conventions. - Assigning
1is redundant. It is also done because of the0.
vkucera
reviewed
Apr 16, 2026
Comment on lines
+547
to
+555
| case 211: | ||
| pid = kPion; | ||
| break; | ||
| case 321: | ||
| pid = kKaon; | ||
| break; | ||
| case 2212: | ||
| pid = kProtonMy; | ||
| break; |
vkucera
reviewed
Apr 16, 2026
| auto pTrack = v0.template posTrack_as<DaughterTracks>(); | ||
| auto nTrack = v0.template negTrack_as<DaughterTracks>(); | ||
|
|
||
| if (0 && v0.v0radius() > v0Radius && |
Collaborator
There was a problem hiding this comment.
What is the 0 doing there?
vkucera
reviewed
Apr 16, 2026
| std::abs(pTrack.eta()) < etaDau && | ||
| std::abs(nTrack.eta()) < etaDau) { | ||
|
|
||
| registry.fill(HIST("hv0mass"), cent, double(kK0short), v0.eta(), double(v0.mK0Short())); |
Collaborator
There was a problem hiding this comment.
Why do you need the casting?
- Convert all C-style anonymous enums to typed enum classes: EventCategory, TriggerClass, Species, TrackSign, AnalysisStep, ParticleType, PtVariation - Add constexpr toInt<E>() helper to convert scoped enum to int for use with registry.fill() and vector indexing - Rename ambiguous enumerators for clarity: kProtonMy -> Proton, kOPar -> Other, kNoPtVar -> None, kPtDw -> Down - Update all usages throughout processData, processMC, and processMCNoV0Counting process functions - Remove dead V0 loop code in processMCV0Counting (parameter fullV0s unused, inner loop had permanent `0 &&` guard) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
vkucera
reviewed
Apr 16, 2026
Comment on lines
+173
to
+175
| // Scoped-enum → int helper | ||
| template <typename E> | ||
| constexpr int toInt(E e) { return static_cast<int>(e); } |
Collaborator
There was a problem hiding this comment.
If you use unscoped enumerations with the prefixes, you don't need all this nonsense that Claude did.
| namespace | ||
| { | ||
| template <typename T> | ||
| static constexpr bool hasCent() |
Comment on lines
+177
to
+198
| // Weights and Fill values magic numbers | ||
| constexpr float W1 = -10.0f; | ||
| constexpr float W2 = 2.0f; | ||
| constexpr float W3 = 5.0f; | ||
| constexpr float W4 = 0.5f; | ||
| constexpr float Fill1 = 1.0f; | ||
| constexpr float Fill2 = 2.0f; | ||
| constexpr float Fill3 = 3.0f; | ||
| constexpr float Fill4 = 4.0f; | ||
| constexpr float Fill5 = 5.0f; | ||
| constexpr float Fill6 = 6.0f; | ||
| constexpr float Fill7 = 7.0f; | ||
| constexpr float Fill9 = 9.0f; | ||
| constexpr float Fill10 = 10.0f; | ||
| constexpr float Fill11 = 11.0f; | ||
| constexpr float Fill12 = 12.0f; | ||
| constexpr float Fill13 = 13.0f; | ||
| constexpr float Fill14 = 14.0f; | ||
| constexpr float Fill15 = 15.0f; | ||
| constexpr float Fill16 = 16.0f; | ||
| constexpr float Fill17 = 17.0f; | ||
| constexpr float FillM1 = -1.0f; |
Collaborator
There was a problem hiding this comment.
These generic constants make no sense. Declare proper named enum constants and use them to label and fill the bins.
Comment on lines
+312
to
+320
| x->SetBinLabel(1, "All"); | ||
| x->SetBinLabel(2, "Sel8"); | ||
| x->SetBinLabel(3, "Sel8z10"); | ||
| x->SetBinLabel(4, "Generated INEL>0"); | ||
| x->SetBinLabel(5, "Good BCs"); | ||
| x->SetBinLabel(6, "BCs with collisions"); | ||
| x->SetBinLabel(7, "BCs with pile-up/splitting"); | ||
| x->SetBinLabel(8, "INEL&Sel8&mcz10"); | ||
| x->SetBinLabel(9, "INEL"); |
Collaborator
There was a problem hiding this comment.
This is super bug-prone to maintain. Use enum.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.